Repository navigation
feat: add natural language log querying - #2
Conversation
📝 WalkthroughWalkthroughRemoves tracked ChangesNatural-Language Query Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
cmd/query/main.go (2)
161-161: 💤 Low valueConsider making the model name configurable.
The model
"llama3.2:3b"is hardcoded. A flag like--modelwould let users swap models without code changes (e.g., for faster inference or different capabilities).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/query/main.go` at line 161, The model name "llama3.2:3b" is hardcoded in the NewOllamaParser function call on line 161. Add a command-line flag (such as --model) to make the model name configurable, retrieve the flag value, and pass it to NewOllamaParser instead of the hardcoded string. This allows users to specify different models when running the command without modifying code.
228-235: ⚡ Quick winAdd "no logs found" feedback for empty results.
The main
GetLogspath (line 96-102) prints "no logs found" whenresp.Logsis nil, butrunNLQsilently exits. For consistency and better UX, handle the empty case.Proposed fix
+ if resp.Logs == nil || len(resp.Logs) == 0 { + fmt.Println("no logs found") + return + } + for _, logEntry := range resp.Logs {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/query/main.go` around lines 228 - 235, The for loop in runNLQ that iterates over resp.Logs does not handle the empty case, whereas the main GetLogs path (lines 96-102) prints "no logs found" when resp.Logs is nil or empty. Add a check before the for loop in runNLQ to print "no logs found" when resp.Logs is empty or nil, ensuring consistent user feedback between both code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/query/main.go`:
- Around line 186-200: The time.ParseDuration function does not support day
units like "7d" that the LLM parser may output, causing silent failures where
the StartTime filter remains unset. Before calling time.ParseDuration on
intent.Since, add preprocessing logic to normalize day units by converting
patterns like "7d" to their equivalent hour format (e.g., "168h") using string
manipulation. This requires parsing the duration string to identify day unit
suffixes and converting them to supported units before passing to
time.ParseDuration. Consider using strings and strconv packages to handle this
conversion, and optionally warn the user if the parsing still fails after
normalization.
In `@internal/nlq/ollama.go`:
- Around line 193-205: After calling client.Do(req) and checking for error, you
must also validate the HTTP status code from the response before attempting to
decode the JSON. Add a check after defer resp.Body.Close() to verify that
resp.StatusCode indicates success (check if it's in the 2xx range). If the
status code indicates an error, return an error immediately with details about
the HTTP status and optionally the response body. Only proceed to the
json.NewDecoder(resp.Body).Decode(&res) call if the status code check passes.
This ensures actual server errors are properly surfaced instead of confusing
JSON unmarshal errors.
- Around line 210-239: The callOllamaAPI function is unused in the codebase and
has an inconsistent JSON field mapping compared to its method counterpart—it
decodes "output" instead of "response", which would cause it to silently return
empty strings when calling Ollama's API. Remove the entire callOllamaAPI
function definition since it is not being called anywhere and relies on the
incorrect JSON field name.
- Around line 160-196: The HTTP client created in the callOllamaAPI function
lacks a timeout configuration, which can cause indefinite blocking if Ollama
becomes unresponsive. Add "time" to the imports at the top of the file, then
modify the client initialization from http.Client{} to include a Timeout field
set to an appropriate duration value (e.g., 30 seconds or similar) using
time.Duration to prevent requests from hanging indefinitely.
- Around line 145-146: Remove the two debug fmt.Println statements that output
raw LLM responses to stdout. Locate the lines containing fmt.Println("RAW OLLAMA
RESPONSE:") and fmt.Println(resp) in the ollama.go file and delete both
statements entirely, as debug output should not be present in production code.
In `@internal/storage/postgres.go`:
- Around line 217-222: Remove the fmt.Printf debug output statement that prints
distance and message fields in the storage layer. This unstructured output
pollutes server stdout and exposes potentially sensitive log content. If debug
logging is needed for development, replace it with a proper structured logging
call at an appropriate debug level that respects the application's logging
configuration, otherwise delete the fmt.Printf block entirely.
---
Nitpick comments:
In `@cmd/query/main.go`:
- Line 161: The model name "llama3.2:3b" is hardcoded in the NewOllamaParser
function call on line 161. Add a command-line flag (such as --model) to make the
model name configurable, retrieve the flag value, and pass it to NewOllamaParser
instead of the hardcoded string. This allows users to specify different models
when running the command without modifying code.
- Around line 228-235: The for loop in runNLQ that iterates over resp.Logs does
not handle the empty case, whereas the main GetLogs path (lines 96-102) prints
"no logs found" when resp.Logs is nil or empty. Add a check before the for loop
in runNLQ to print "no logs found" when resp.Logs is empty or nil, ensuring
consistent user feedback between both code paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee5da874-e538-4ea0-83e1-f33d84d12474
📒 Files selected for processing (9)
.idea/.gitignore.idea/go.imports.xml.idea/loom.iml.idea/modules.xml.idea/vcs.xmlcmd/query/main.gointernal/nlq/intent.gointernal/nlq/ollama.gointernal/storage/postgres.go
💤 Files with no reviewable changes (5)
- .idea/loom.iml
- .idea/go.imports.xml
- .idea/vcs.xml
- .idea/modules.xml
- .idea/.gitignore
| if intent.Since != "" { | ||
|
|
||
| duration, err := time.ParseDuration( | ||
| intent.Since, | ||
| ) | ||
|
|
||
| if err == nil { | ||
|
|
||
| start := time.Now(). | ||
| Add(-duration). | ||
| Unix() | ||
|
|
||
| filter.StartTime = &start | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Go time.ParseDuration support day units like 7d?
💡 Result:
No, Go's time.ParseDuration function does not support "d" (day) or any units larger than hours [1][2][3]. The valid unit suffixes supported by time.ParseDuration are [1][2]: - ns (nanoseconds) - us or µs (microseconds) - ms (milliseconds) - s (seconds) - m (minutes) - h (hours) The Go language designers intentionally omitted units for days or larger intervals to avoid the ambiguity caused by daylight savings time transitions and varying day lengths [4][5][6]. If you need to represent a duration in days, you should convert it to hours (e.g., use "72h" instead of "3d" or "7d") [3][5].
Citations:
- 1: https://pkg.go.dev/time@go1.26.4
- 2: https://go.dev/src/time/format.go?m=text
- 3: https://stackoverflow.com/questions/75853288/how-to-mention-time-duration-in-days-so-that-golang-viper-config-can-be-loaded-w
- 4: https://pkg.go.dev/time@go1.26.3
- 5: flag: let Duration accept days golang/go#11473
- 6: https://go.dev/src/time/time.go?m=text
🏁 Script executed:
# First, check if the file exists and inspect the relevant lines
cd /
find . -name "main.go" -path "*/cmd/query/*" 2>/dev/null | head -5Repository: Aneeshie/Loom
Length of output: 95
🏁 Script executed:
# Count lines in the file to understand its size
wc -l cmd/query/main.goRepository: Aneeshie/Loom
Length of output: 79
🏁 Script executed:
# Read the specific section mentioned (lines 186-200)
sed -n '186,200p' cmd/query/main.goRepository: Aneeshie/Loom
Length of output: 255
🏁 Script executed:
# Check the broader context around runNLQ function and the LLM prompt
sed -n '155,236p' cmd/query/main.goRepository: Aneeshie/Loom
Length of output: 1166
🏁 Script executed:
# Check the imports to see what's available
sed -n '1,30p' cmd/query/main.goRepository: Aneeshie/Loom
Length of output: 931
time.ParseDuration does not support day units ("7d").
The LLM parser likely outputs durations like "7d" for natural language queries such as "last week", but Go's time.ParseDuration only supports ns, us, ms, s, m, h. When parsing fails, the error is silently ignored (lines 192–193), causing the StartTime filter to remain unset.
Either normalize day units (e.g., convert "7d" to "168h") before parsing or warn the user when parsing fails.
Proposed fix — convert days to hours before parsing
if intent.Since != "" {
+ // Normalize day units to hours (time.ParseDuration doesn't support "d")
+ since := intent.Since
+ if strings.HasSuffix(since, "d") {
+ daysStr := strings.TrimSuffix(since, "d")
+ if days, err := strconv.Atoi(daysStr); err == nil {
+ since = fmt.Sprintf("%dh", days*24)
+ }
+ }
duration, err := time.ParseDuration(
- intent.Since,
+ since,
)
- if err == nil {
-
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "warning: could not parse duration %q: %v\n", intent.Since, err)
+ } else {
start := time.Now().
Add(-duration).
Unix()
-
filter.StartTime = &start
}
}Add imports for strconv, strings, and os.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/query/main.go` around lines 186 - 200, The time.ParseDuration function
does not support day units like "7d" that the LLM parser may output, causing
silent failures where the StartTime filter remains unset. Before calling
time.ParseDuration on intent.Since, add preprocessing logic to normalize day
units by converting patterns like "7d" to their equivalent hour format (e.g.,
"168h") using string manipulation. This requires parsing the duration string to
identify day unit suffixes and converting them to supported units before passing
to time.ParseDuration. Consider using strings and strconv packages to handle
this conversion, and optionally warn the user if the parsing still fails after
normalization.
| fmt.Println("RAW OLLAMA RESPONSE:") | ||
| fmt.Println(resp) |
There was a problem hiding this comment.
Remove debug print statements.
These fmt.Println calls output raw LLM responses to stdout in production, which pollutes CLI output and may leak user query context.
Proposed fix
- fmt.Println("RAW OLLAMA RESPONSE:")
- fmt.Println(resp)
-
var intent QueryIntent📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Println("RAW OLLAMA RESPONSE:") | |
| fmt.Println(resp) | |
| var intent QueryIntent |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/nlq/ollama.go` around lines 145 - 146, Remove the two debug
fmt.Println statements that output raw LLM responses to stdout. Locate the lines
containing fmt.Println("RAW OLLAMA RESPONSE:") and fmt.Println(resp) in the
ollama.go file and delete both statements entirely, as debug output should not
be present in production code.
| func (p *OllamaParser) callOllamaAPI( | ||
| ctx context.Context, | ||
| prompt string, | ||
| ) (string, error) { | ||
|
|
||
| client := &http.Client{} | ||
|
|
||
| payload := map[string]any{ | ||
| "model": p.model, | ||
| "prompt": prompt, | ||
| "stream": false, | ||
| } | ||
|
|
||
| body, err := json.Marshal(payload) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| req, err := http.NewRequestWithContext( | ||
| ctx, | ||
| "POST", | ||
| "http://localhost:11434/api/generate", | ||
| bytes.NewBuffer(body), | ||
| ) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| req.Header.Set( | ||
| "Content-Type", | ||
| "application/json", | ||
| ) | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Add HTTP client timeout to prevent hanging on unresponsive Ollama.
The HTTP client has no timeout configured. LLM inference can be slow, and if Ollama is unresponsive, this will block indefinitely without any indication to the user.
Proposed fix
func (p *OllamaParser) callOllamaAPI(
ctx context.Context,
prompt string,
) (string, error) {
- client := &http.Client{}
+ client := &http.Client{
+ Timeout: 120 * time.Second,
+ }Add "time" to the imports.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/nlq/ollama.go` around lines 160 - 196, The HTTP client created in
the callOllamaAPI function lacks a timeout configuration, which can cause
indefinite blocking if Ollama becomes unresponsive. Add "time" to the imports at
the top of the file, then modify the client initialization from http.Client{} to
include a Timeout field set to an appropriate duration value (e.g., 30 seconds
or similar) using time.Duration to prevent requests from hanging indefinitely.
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| var res struct { | ||
| Response string `json:"response"` | ||
| } | ||
|
|
||
| if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Check HTTP status code before decoding response.
If Ollama returns an error (4xx/5xx), the code attempts JSON decode anyway, producing a confusing unmarshal error instead of surfacing the actual failure.
Proposed fix
resp, err := client.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
+ return "", fmt.Errorf("ollama API error: status %d", resp.StatusCode)
+ }
+
var res struct {
Response string `json:"response"`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resp, err := client.Do(req) | |
| if err != nil { | |
| return "", err | |
| } | |
| defer resp.Body.Close() | |
| var res struct { | |
| Response string `json:"response"` | |
| } | |
| if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { | |
| return "", err | |
| } | |
| resp, err := client.Do(req) | |
| if err != nil { | |
| return "", err | |
| } | |
| defer resp.Body.Close() | |
| if resp.StatusCode != http.StatusOK { | |
| return "", fmt.Errorf("ollama API error: status %d", resp.StatusCode) | |
| } | |
| var res struct { | |
| Response string `json:"response"` | |
| } | |
| if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { | |
| return "", err | |
| } |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 197-197: Error return value of resp.Body.Close is not checked
(errcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/nlq/ollama.go` around lines 193 - 205, After calling client.Do(req)
and checking for error, you must also validate the HTTP status code from the
response before attempting to decode the JSON. Add a check after defer
resp.Body.Close() to verify that resp.StatusCode indicates success (check if
it's in the 2xx range). If the status code indicates an error, return an error
immediately with details about the HTTP status and optionally the response body.
Only proceed to the json.NewDecoder(resp.Body).Decode(&res) call if the status
code check passes. This ensures actual server errors are properly surfaced
instead of confusing JSON unmarshal errors.
| func callOllamaAPI(ctx context.Context, prompt string) (string, error) { | ||
| client := &http.Client{} | ||
| payload := map[string]string{ | ||
| "model": "llama3.2:3b", | ||
| "prompt": prompt, | ||
| } | ||
| body, _ := json.Marshal(payload) | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, "POST", "http://localhost:11434/api/generate", bytes.NewBuffer(body)) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| var res struct { | ||
| Output string `json:"output"` | ||
| } | ||
|
|
||
| if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return res.Output, nil | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Remove unused callOllamaAPI function.
This standalone function is unused and inconsistent with the method version—it decodes "output" (line 231) instead of "response" (line 200), which would silently return empty strings from Ollama's actual API.
Proposed fix
-func callOllamaAPI(ctx context.Context, prompt string) (string, error) {
- client := &http.Client{}
- payload := map[string]string{
- "model": "llama3.2:3b",
- "prompt": prompt,
- }
- body, _ := json.Marshal(payload)
-
- req, err := http.NewRequestWithContext(ctx, "POST", "http://localhost:11434/api/generate", bytes.NewBuffer(body))
- if err != nil {
- return "", err
- }
- req.Header.Set("Content-Type", "application/json")
-
- resp, err := client.Do(req)
- if err != nil {
- return "", err
- }
- defer resp.Body.Close()
-
- var res struct {
- Output string `json:"output"`
- }
-
- if err := json.NewDecoder(resp.Body).Decode(&res); err != nil {
- return "", err
- }
-
- return res.Output, nil
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func callOllamaAPI(ctx context.Context, prompt string) (string, error) { | |
| client := &http.Client{} | |
| payload := map[string]string{ | |
| "model": "llama3.2:3b", | |
| "prompt": prompt, | |
| } | |
| body, _ := json.Marshal(payload) | |
| req, err := http.NewRequestWithContext(ctx, "POST", "http://localhost:11434/api/generate", bytes.NewBuffer(body)) | |
| if err != nil { | |
| return "", err | |
| } | |
| req.Header.Set("Content-Type", "application/json") | |
| resp, err := client.Do(req) | |
| if err != nil { | |
| return "", err | |
| } | |
| defer resp.Body.Close() | |
| var res struct { | |
| Output string `json:"output"` | |
| } | |
| if err := json.NewDecoder(resp.Body).Decode(&res); err != nil { | |
| return "", err | |
| } | |
| return res.Output, nil | |
| } |
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 228-228: Error return value of resp.Body.Close is not checked
(errcheck)
[error] 210-210: func callOllamaAPI is unused
(unused)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/nlq/ollama.go` around lines 210 - 239, The callOllamaAPI function is
unused in the codebase and has an inconsistent JSON field mapping compared to
its method counterpart—it decodes "output" instead of "response", which would
cause it to silently return empty strings when calling Ollama's API. Remove the
entire callOllamaAPI function definition since it is not being called anywhere
and relies on the incorrect JSON field name.
Source: Linters/SAST tools
| fmt.Printf( | ||
| "distance=%.4f message=%s\n", | ||
| distance, | ||
| logEntry.Message, | ||
| ) | ||
|
|
There was a problem hiding this comment.
Remove server-side debug output.
This fmt.Printf runs in the storage layer for every matched log, polluting server stdout and potentially exposing sensitive log message content. Debug output should use structured logging with appropriate levels, or be removed before merge.
Proposed fix
if distance > SIMILARITY_THRESHOLD {
continue
}
- fmt.Printf(
- "distance=%.4f message=%s\n",
- distance,
- logEntry.Message,
- )
-
logs = append(logs, logEntry)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fmt.Printf( | |
| "distance=%.4f message=%s\n", | |
| distance, | |
| logEntry.Message, | |
| ) | |
| if distance > SIMILARITY_THRESHOLD { | |
| continue | |
| } | |
| logs = append(logs, logEntry) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/storage/postgres.go` around lines 217 - 222, Remove the fmt.Printf
debug output statement that prints distance and message fields in the storage
layer. This unstructured output pollutes server stdout and exposes potentially
sensitive log content. If debug logging is needed for development, replace it
with a proper structured logging call at an appropriate debug level that
respects the application's logging configuration, otherwise delete the
fmt.Printf block entirely.
Overview
This PR introduces natural language querying for Loom.
Users can now search logs using plain English instead of manually constructing filters and semantic search queries.
Examples
The query is interpreted by an LLM and translated into a structured query intent, which is then executed using Loom's existing filtering and semantic search pipeline.
Architecture
Example:
↓
{ "query": "authentication", "level": "ERROR", "service": "", "host": "", "since": "" }↓
↓
Changes
Added
internal/nlqpackageQueryIntentmodelIntentParserinterfaceOllamaParserimplementation--askCLI flagSupported Intent Fields
{ "query": "", "level": "", "service": "", "host": "", "since": "" }Query Routing
SimilarLogs()when a semantic query is presentGetLogs()for filter-only queriesDesign Goals
Example Queries
↓
{ "query": "authentication", "level": "ERROR" }↓
{ "query": "database", "level": "ERROR", "since": "1h" }↓
{ "query": "", "level": "WARN", "service": "auth" }Future Improvements
Result
Loom now supports querying logs using plain English while reusing the existing filtering and hybrid semantic search infrastructure.
Summary by CodeRabbit
New Features
-askCLI flag, enabling users to ask questions about logs in plain English.Chores